-
Notifications
You must be signed in to change notification settings - Fork 7.3k
module: fix column offset on first line stack trace #25342
Conversation
When modules are loaded by require(), they are wrapped in a function. This added text causes errors on the first line of a file to be offset by the length of the function declaration. For example, an error on the first character of a module would appear as if it were on column 63. To fix this problem I added line and column offset support to the ContextifyScript object and added options to the runInContext(and friends) function. I then added a column offset to module._compile that compensates for the function wrapper. Fixes nodejs#9445
if (args[i]->IsInt32()) { | ||
return args[i].As<Integer>(); | ||
} | ||
if (!args[i]->IsObject()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the only check you actually need here. The IsUndefined()
and IsInt32()
checks above can go. Same comment applies for your GetColumnOffsetArg()
function.
@joyent/node-coreteam what do you think of this? It does seem to solve the problem. If we take this, I'm not sure we should necessarily document it as part of the public API. Also, if we go this route, an alternative would be to change the module wrapper from:
to
And just use a |
@@ -441,8 +441,10 @@ Module.prototype._compile = function(content, filename) { | |||
|
|||
// create wrapper function | |||
var wrapper = Module.wrap(content); | |||
var wrapperOffset = Module.wrapper[0].length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be pulled out of the function to avoid calculating every time.
@brendan0powers ... how would you like to proceed on this? @cjihrig raised a number of issues that would need to be addressed. If you'd like to pursue, updating the PR and re-opening it against http://github.com/nodejs/node would likely be the best approach. |
It looks like this is being followed up in nodejs/node#2867. I'll close this for now, thanks! |
When modules are loaded by require(), they are wrapped in
a function. This added text causes errors on the first line
of a file to be offset by the length of the function
declaration. For example, an error on the first character
of a module would appear as if it were on column 63.
To fix this problem I added line and column offset support
to the ContextifyScript object and added options to the
runInContext(and friends) function. I then added a column
offset to module._compile that compensates for the function
wrapper.
Fixes #9445